-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass "certificate policies" extension to callback #3419
Pass "certificate policies" extension to callback #3419
Conversation
I'm afraid we can't remove this option now. That would be considered an incompatible change and we don't do that in minor versions of Mbed TLS. (We might make an exception for security, and even so, we still haven't removed the transition option Since we plan to release Mbed TLS 3.0 this year, I don't think it's worth it to touch |
Ok thanks. I understand. What about the handling of unsupported policies? Currently the "certificate policies" extension is known by mbedTLS and therefore never passed to the callback, even if it contains unsupported policies. I am not sure how common these policies are - do you think there is any value in making the callback handle them as I did in this PR, even if the deprecated option isn't removed? |
Passing some known extensions to the callback does make sense. It contradicts the current documentation for I wonder if this should be just for certificatePolicy with unknown policies, or possibly for other extensions? Maybe the documentation of |
My view was only those that being potentially critical can make the parsing fail.
I already changed it tn the PR to reflect the change I made: https://github.com/ARMmbed/mbedtls/pull/3419/files#diff-97f260303207ad6227a973dbe90fcc39
I don't particularly like the sound of that, but I'm not sure. |
I have reworked and force-pushed this. The deprecated option is no longer removed and I've only kept the passing of "certificate policies" to the callback if it contains unsupported policies. |
Pass the "certificate policies" extension to the callback supplied to mbedtls_x509_crt_parse_der_with_ext_cb() if it contains unsupported policies. This allows the callback to fully replicate the behaviour of the deprecated MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION configuration. Signed-off-by: Nicola Di Lieto <[email protected]>
as suggested in Mbed-TLS#3419 (comment) also removed two no longer necessary void casts Signed-off-by: Nicola Di Lieto <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments. LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me. But please add a note about possible API evolution.
as requested, see Mbed-TLS#3419 (comment) Signed-off-by: Nicola Di Lieto <[email protected]>
Description
Pass the "certificate policies" extension to the callback supplied to mbedtls_x509_crt_parse_der_with_ext_cb() if it contains unsupported policies.
This allows the callback to fully replicate the behaviour of the deprecated MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION configuration.
Signed-off-by: Nicola Di Lieto [email protected]
Status
READY
Requires Backporting
NO